Pull request overview

Adds support for running multiple log providers concurrently within a single context-scribe daemon instance, using a shared evaluator + Memory Bank client and an interaction fan-in queue.

Changes:

• Introduces a TOOL_REGISTRY and _create_providers() to bootstrap/instantiate providers from tool names.
• Adds --tools (CSV) CLI option and updates the daemon loop to run one watcher task per provider, feeding a shared asyncio.Queue.
• Adds new unit tests covering registry population and provider creation behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Automated review — NEEDS HUMAN REVIEW

Risk: MEDIUM
Reviewed commit: d15609dabf906426a9c8a758429c6d322913d9f6
Council vote: security=MEDIUM · correctness=MEDIUM · maintainability=MEDIUM

Summary

The PR implements multi-tool concurrent daemon support (issue #12) cleanly — the TOOL_REGISTRY pattern, async queue fan-in, and 75 passing tests all look solid, and no security-sensitive surfaces were touched. However, the PR cannot be merged in its current state: the branch has conflicts with main (DIRTY merge state). Beyond the merge conflict, two correctness issues need attention: watcher tasks are cancelled but never awaited in the shutdown finally block (risk of use-after-close races), and generator cleanup relies on __del__ rather than explicit close(). Documentation also lags the implementation — CONTRIBUTING.md still describes the old if/elif tool-registration flow that the new TOOL_REGISTRY has replaced.

Linked issue analysis

Issue #12 — multi-tool concurrent daemon support: All acceptance criteria are addressed. The --tools CSV option is wired, TOOL_REGISTRY replaces the old per-tool dispatch, providers run as independent watcher tasks sharing a fan-in queue, and the shared evaluator + Memory Bank client are reused across tools. Tests cover registry population, provider creation (single/multi/unknown/bootstrap), and the daemon lifecycle.

Findings

MAJOR

• [major] (branch-level) — Merge conflict (correctness · maintainability): PR is in CONFLICTING / DIRTY state against main. Must be rebased before it can land.
• [major] context_scribe/main.py:336 — Async lifecycle (correctness): Watcher tasks are cancelled via task.cancel() but never awaited in the finally block. mcp_client.close() proceeds immediately, risking use-after-close errors and leaving CancelledError exceptions unobserved. Fix: await asyncio.gather(*tasks, return_exceptions=True) after cancellation.
• [major] CONTRIBUTING.md — Documentation drift (maintainability): CONTRIBUTING.md still instructs contributors to add tools via the old if/elif logic in run_daemon. The new TOOL_REGISTRY registration pattern replaces that workflow but is entirely undocumented.

MINOR

• [minor] context_scribe/main.py:263 — Resource leak (correctness): Generator cleanup for provider.watch() relies on __del__ after task cancellation. Python does not guarantee prompt __del__, which can leak watchdog Observer threads. Fix: explicitly call watch_iter.close() in the cancel/finally path.
• [minor] context_scribe/main.py:264 — Deprecated API (correctness · maintainability): asyncio.get_event_loop() called inside async functions _watch_provider() and _loop(). On Python 3.10+ (this project targets 3.12), use asyncio.get_running_loop() from a running coroutine.
• [minor] context_scribe/main.py:242 — Evaluator selection (correctness): _detect_evaluator(tool_names[0]) uses only the first tool when multiple tools are provided. --tools gemini-cli,claude would select a Gemini evaluator for Claude interactions, producing incorrect rule extraction.
• [minor] README.md — Documentation drift (maintainability): Usage section shows only --tool examples; the new --tools multi-tool option is not mentioned.
• [minor] context_scribe/main.py — Missing type hints (maintainability): _create_providers has no return type annotation; _watch_provider's provider parameter has no type hint. CONTRIBUTING.md requires type hints for all function signatures.

INFO

• [info] context_scribe/main.py — --tools validated against TOOL_REGISTRY keys with dedup and empty-input rejection; no injection surface. (security)
• [info] context_scribe/main.py — asyncio.Queue(maxsize=1000) bounds memory growth from provider watchers. (security)
• [info] (PR comment) — Open collaborator question from don-petry (2026-04-05) has not been answered. (correctness)
• [info] tests/test_daemons.py — test_run_daemon_tools patches bootstrap_global_config even though _create_providers is fully mocked, making the bootstrap patch a no-op. (correctness · maintainability)
• [info] context_scribe/main.py — from typing import List used; prefer built-in list[str] on Python 3.10+. (maintainability)
• [info] tests/test_multi_tool.py — test_create_providers_calls_bootstrap mutates TOOL_REGISTRY directly instead of patch.dict; safer against unexpected exceptions. (maintainability)
• [info] context_scribe/main.py — _detect_evaluator is called with tool_names[0] when multiple tools are provided — a comment explaining this choice would help future maintainers. (maintainability)

CI status

2 checks passing (Detect ecosystems, pip-audit), 4 skipped (dependabot, npm audit, govulncheck, cargo audit). No test-suite check appears in the rollup for this commit — branch is CONFLICTING / DIRTY against main, so merge-commit verification has not run. All 75 unit tests passed on the PR branch head per test logs.

Reviewed automatically by the don-petry PR-review council (security: opus 4.6 · correctness: sonnet 4.6 · maintainability: sonnet 4.6 · synthesis: sonnet 4.6). The marker on line 1 lets the agent detect new commits and re-review. Reply with @don-petry if you need a human.